-
Notifications
You must be signed in to change notification settings - Fork 89
Refactor base types #195
Refactor base types #195
Conversation
3d346cb
to
1e17c2a
Compare
Hm. Seems I got some issues to fix:
|
e3b2b76
to
e989c59
Compare
058810b
to
6c038a8
Compare
`T` always was the same as `Self`. Also do a major bump on depending crates.
… in implementations (apart from Debug) - where clauses are almost never needed on type definitions unless you need them in the `Drop` implementation - Debug trait might be useful if (debug) logging gets added - Clone/PartialEq/Eq shouldn't be needed ever in the implementations
6c038a8
to
804c762
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow this is great!
The Codec
thing has been bothering me from the start, and I didn't know how to improve the situation. I also love the AsyncSocket
trait. I'm slightly bothered by making the netlink-sys
API rely on bytes
but the improvement is really worth it. Also, thank you for the soundness fix.
I'm going to let this open one day or two in case someone else wants to take a look, but I love it!
1. get rid of "workaround-audit-bug" "feature" 2. no longer use tokio_util::codec::{Decoder, Encoder} tokio_util::codec is "designed" for bytestreams (and building "frames" of messages on top), but we need to deal with datagrams (which still can contain multiples messages, just not one message across multiple datagrams). This make it a little less "combinable", but we actually don't want people to reuse these codecs on bytestream (and writing an adapter wouldn't be that hard anyway). Also we can use a fixed error type, making dealing with it a little bit easier.
Creating &mut [u8] (and &[u8]) for unitialized memory is undefined behaviour even when not actually reading the data. Use bytes::BufMut instead, and advance buffer in recv functions. High level functions (without flags param) don't need to return length of read, as it was used to advance the buffer - only low-level read might return larger length than the buffer (PEEK + TRUNC flags). This also bumps netlink-sys; the other crates already got bumped.
Preparation to make tokio and smol feature non-conflicting in netlink-proto.
- This adds a new type paramater to connection for the socket being used. - can drop tokio/smol selection from netlink-packet-audit
804c762
to
aa89fe6
Compare
Thanks @stbuehler Do you want me to make new releases after this is merged, or do you want to rebase #170 and have releases once we have batch messages support? |
Wanted to write a few words about that:
I don't need a release to continue working on #170 - I'd say that is up to you :) |
Sounds good. If you have links to specific RFCs/PRs or issues I'm interested! For now, let's get this in! I'll make a release after you've rebased #170 Thanks for the quality work :) |
Hi again.
#170 currently is broken because the
workaround-audit-bug
feature does bad things (it basically ignores the message length, which means the payload suddenly also contains the padding in my "test payload withlength % 4 != 0
" tests)So I tried to get rid of that "feature" (also mentioned in #141 as "todo item"), and added
Codec
as type parameter toConnection
.As this is a major change anyway I tried to cleanup a few other things; those 4 commits should mostly be unrelated (I recommend reviewing them individually), so the subject of this PR just says "refactor" - but they all are API changes, so it makes somewhat sense to combine them (imho).
If this gets accepted I'm gonna basically have to redo #170 - so I'd like to get some feedback before I'll go back to #170 :)